Add singleton-dimension edge-case tests for exported _data() functions#521
Add singleton-dimension edge-case tests for exported _data() functions#521utkarshpawade wants to merge 7 commits intostan-dev:masterfrom
_data() functions#521Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #521 +/- ##
=======================================
Coverage 98.67% 98.67%
=======================================
Files 35 35
Lines 5905 5905
=======================================
Hits 5827 5827
Misses 78 78 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jgabry
left a comment
jgabry
left a comment
There was a problem hiding this comment.
Thanks. A bunch of comments:
-
I don't think we should have an entirely separate test file just for edge cases. I would put tests for these functions together with the existing tests for these functions.
-
Regarding the singleton-dimension tests: these are good and they're worth keeping but they should go with the other tests for these functions not in a separate file.
-
Regarding the tests for empty inputs: the exported
_data()helpers do not currently have a consistent empty (zero length) input behavior: some return empty tibbles,ppc_bars_data()errors, andppd_stat_data()currently fails with a low-level row-mismatch error. The tests here enforce this inconsistent behavior. I think we should first standardize the behavior (we'd need to decide on what it should be, that could be handled separately from this PR, we could discuss in an issue) and only then add tests. I'm glad you added these tests even though I don't want to keep them because it made me realize that this behavior isn't standardized across the functions! -
Regarding the
ppc_loo_pit_data()additions: this feels over-specified relative to the value it adds. We already have coverage for bothboundary_correctionpaths and the returned structure. I’d keep one focused test for thepit =shortcut path and remove the repeatedexpect_message(..., "pit")/is_y_labelchecks for now.
jgabry
changed the title
_data() functions
jgabry
left a comment
jgabry
left a comment
There was a problem hiding this comment.
Thanks, looks good, just one small change to make
|
|
||
| yrep_1obs <- matrix(rnorm(5), ncol = 1) | ||
| d2 <- ppd_stat_data(yrep_1obs, stat = "mean") | ||
| expect_s3_class(d2, "data.frame") |
There was a problem hiding this comment.
Can you check nrow(d2) like you already check nrow(d) above?
There was a problem hiding this comment.
Pull request overview
Adds explicit singleton-dimension edge-case unit tests for exported PPC/PPD *_data() helpers, addressing gaps noted in #520 where these user-facing data-prep functions were only tested indirectly via plotting wrappers.
Changes:
- Added tests for single observation and/or single draw cases across multiple
*_data()helpers (scatter, error, intervals, bars, stat, ppd/loo PIT). - Added a focused test for
ppc_bars_data(prob = 0)interval-collapse behavior. - Updated
NEWS.mdto record the added edge-case test coverage.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/testthat/test-ppc-test-statistics.R | Adds singleton tests for ppd_stat_data() (single draw / single observation). |
| tests/testthat/test-ppc-scatterplots.R | Adds singleton tests for ppc_scatter_data() and ppc_scatter_avg_data(). |
| tests/testthat/test-ppc-loo.R | Adds a single-value PIT test for ppc_loo_pit_data(). |
| tests/testthat/test-ppc-intervals.R | Adds singleton tests for ppd_intervals_data() (single observation / single draw). |
| tests/testthat/test-ppc-errors.R | Adds single-observation test for ppc_error_data(). |
| tests/testthat/test-ppc-distributions.R | Adds single-observation test for ppd_data(). |
| tests/testthat/test-ppc-discrete.R | Adds singleton tests for ppc_bars_data() and prob = 0 behavior. |
| NEWS.md | Notes the addition of singleton-dimension edge-case tests for exported *_data() functions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fixes #520
Summary
Adds singleton-dimension edge-case tests for exported _data() functions